-
Notifications
You must be signed in to change notification settings - Fork 119
Add support for irreducible CFG #1032
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Thanks for putting up this PR. Is it the case that with this change we will never see the IrreducibleLoopException? |
|
If you need to duplicate a node that has invariants, do you duplicate the invariants as well? I suppose yes and that this will happen naturally. |
Yes, in this case the invariant gets copied too. However, we first attempt to duplicate a node that does not have invariants |
In the current form, I don't think it can be simply dropped. Actually, this is not the exception that was triggered before by a standard run of boogie on an Irreducible loop:
I managed to trigger it by using the boogie/Source/Core/AST/Program.cs Line 558 in 996947e
And caught here (resolved by loop extraction): boogie/Source/VCGeneration/LoopExtractor.cs Lines 13 to 31 in 996947e
The part that I am mostly concerned about is the comment from above: Used by Corral and Dafny.. I believe that one option is to refactor Dafny and Corral to use our implementation of node splitting, but not sure if it's worth it. |
|
|
In general, I would like to take maximal advantage of your impressive PR in Boogie by eliminating all occurrences of
and
If you could do that as part of this PR, great! Otherwise, let us land this PR and hopefully somebody can pick up the remainder of the work later. |
This shouldn't be a problem, I will do it in the coming days (shouldn't take long). |
When I first checked the boogie/Source/Core/AST/Implementation.cs Lines 634 to 640 in 35aebbe
From my understanding, this is part of the Civl infrastructure, thus I assume that in this case the program should be "natively" structured and this piece of code shouldn't be changed. Is this correct? |
Yes, this is part of Civl. But we can change it so that if we discover irreducible loop, we should apply your transformation to make it reducible and then invoke |
34ed7da to
a225a98
Compare
|
@shazqadeer thank you for the review comments and approval. Would you like me to add/change anything else or shall we land it? |
|
Please go ahead and land the PR. |
|
Unfortunately I don't have push rights on master :) |
This PR addresses Issue #921.
Added:
Conversion of irreducible flow graphs by Node Splitting (see Dragon book section 9.7.6). For every irreducible loop found, we duplicate one of the nodes (we first try to duplicate one that does not have invariants, if that exists) and all of the nodes that it dominates.
Tests:
The former Irreducible.bpl test from test0 has been dropped and a new dedicated directory has been created: irreduciblecfg. 6 other test-cases have been added: